Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added CL/CV Support and tests #19

Merged
merged 27 commits into from Jan 28, 2020
Merged

Added CL/CV Support and tests #19

merged 27 commits into from Jan 28, 2020

Conversation

flabbet
Copy link
Member

@flabbet flabbet commented Jan 22, 2020

I added a CL/CV (Travis CL, code Codecov) support and one test, that passes.

EDIT: Set up test enviroment with all enviroment variables. I am currently writing more tests

@pep8speaks
Copy link

pep8speaks commented Jan 22, 2020

Hello @flabbet! Thanks for updating this PR.

Line 56:66: E126 continuation line over-indented for hanging indent

Comment last updated at 2020-01-27 20:25:34 UTC

@codecov
Copy link

codecov bot commented Jan 22, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@081a7b6). Click here to learn what that means.
The diff coverage is 41.46%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master      #19   +/-   ##
=========================================
  Coverage          ?   55.39%           
=========================================
  Files             ?        3           
  Lines             ?      343           
  Branches          ?       46           
=========================================
  Hits              ?      190           
  Misses            ?      146           
  Partials          ?        7
Impacted Files Coverage Δ
swaglyrics_backend/utils.py 49.25% <33.33%> (ø)
swaglyrics_backend/issue_maker.py 56.72% <44.82%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 081a7b6...7c82795. Read the comment docs.

@flabbet
Copy link
Member Author

flabbet commented Jan 22, 2020

Wrote 5 tests. Tomorrow I'll write some more

@flabbet
Copy link
Member Author

flabbet commented Jan 24, 2020

I covered most of main functions, except ones that use @request_from_github() decorator. I am just not successful with patching this. But I still think this is a good start

@@ -50,10 +50,10 @@

SQLALCHEMY_DATABASE_URI = "mysql+mysqlconnector://{username}:{password}@{username}.mysql.pythonanywhere-services." \
"com/{username}${databasename}".format(
username=username,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revert this maybe? formatting changes interfere with reviewing the main code

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's still slightly different :P

setup.py Outdated Show resolved Hide resolved
def test_that_title_mismatches(self):
from swaglyrics_backend.issue_maker import is_title_mismatched
self.assertTrue(
is_title_mismatched({"Bohemian", "Rhapsody", "by", "Queen"}, "Miracle by Caravan Palace", 2))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you pass a set instead of a normal string?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check is_title_mismatched function, it accepts words as first parameter and full title as second

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right so actually we call split() which returns a list. The thing with set is that every element should be unique which may not exist for some songs. You should use a list here to mimic as close to the actual functioning

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

words = title.split()

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't know that set must have unique values, I'll change that

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup.
image

@flabbet
Copy link
Member Author

flabbet commented Jan 27, 2020

Fixed merge conflict. But somehow codecov broke. Logs look normal, idk why. I don't think it's my fault.

EDIT: It fixed itself somehow

@aadibajpai aadibajpai merged commit 729a0d6 into SwagLyrics:master Jan 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants